This is great! Thanks for this. :) You do have some conflicts though. I also left a couple of tiny comments.

Oh and can you also add it to FreeT? :)

FreeT already has mapK a la hoist. I'll add mapK and deprecate hoist. Additionally, I'll add mapK to the yoneda duals.

Codecov Report

Merging #1987 into master will decrease coverage by 0.09%.
The diff coverage is 70.58%.

@@            Coverage Diff            @@
##           master    #1987     +/-   ##
=========================================
- Coverage   96.21%   96.12%   -0.1%     
=========================================
  Files         272      272             
  Lines        4627     4640     +13     
  Branches      115      117      +2     
=========================================
+ Hits         4452     4460      +8     
- Misses        175      180      +5

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b4814a...d623055. Read the comment docs.

I've marked a few items as deprecated. If we'd like to keep the deprecation annotations, I'll need to know the release this PR is scheduled for and update them accordingly.

Can we not have a typeclass here? Seems like we want the analog to Functor no? Any reason not to add with so many instances? The composition law is pretty obvious. Seems worth it to me.

Definitely. There has been related discussion in the past, starting with #1713.

I'd like to add a FunctorK in the same manner as @kailuowang's mainecoon: https://github.com/kailuowang/mainecoon/blob/master/core/src/main/scala/mainecoon/FunctorK.scala.

https://github.com/andyscott/cats/pull/1/files has FunctorK and instances for OptionT and EitherT. If that looks good, I'll give the same treatment to the rest of the instances.

A few notes on my implementation:

For a given transformer, FunctorK is invariant with regards to all non-effect parameters and requires no additional evidence. At the value level, only one instance is needed-- so I use a cast to specialize for non-effect parameters. This can be switched to the traditional implementation if desired.

There's no way easy way to provide arbitrary FunctionK transformations, so the laws for FunctorK just ask for specific transformations to test. In Mainecoon, laws ask for arbitrary instances but elsewhere Gen.const based instances are provided. If we have to use constant instances, I would prefer to pass them in as parameters for clarity.

There are two contention points IRT the type class,

1. what does such an abstraction buy us? In another word, is there a realworld scenario where someone would need a def something[A[_[_]]: FunctorK] ?
2. should the type class be of A[_[_]] or A[_[_], _]? A[_[_]] does cover more types, but due to the limitation of Scala syntax, it has a shortcoming, that is you cannot fix the effect type F[_] inside of it and get a type constructor. A[F] will have to be a concrete type, it cannot be a type constructor. On the other hand, A[_[_], _] can still be a type constructor even if you fix the effect type, since you can write A[F, _]. This gives you the ability to lift a F[_] ~> G[_] to A[F, _] ~> A[G, _], which makes it more like a higher kinded endofunctor with a fmap on FunctionK, and possibly enable more interesting operations like possibly composeK, etc.

On the other hand, both definitions of this type classes ( A[_[_]] or A[_[_], _]) probably don't matter that much because, again, there is no real use case for the abstraction anyway, the only use case is for people to call mapK on it.
To me the main incentive of adding such a type class would be

1. having a unified law testing code and
2. some possibly beneficial utility functions in the type class such as liftK: F[_] ~> G[_] => A[F, _] ~> A[G, _] and composeK: (FunctorK[A[_[_], _]], FunctorK[B[_[_], _]]) => FunctorK[λ[(a[_], b) => A[B[a, _], b]]].

@andyscott what's your incentive for this to be in the same manner as in mainecoon? to support final tagless encoding or do you have some other use cases?

Update: In mainecoon, there is a use case for the FunctorK[A[_[_]] abstraction. I have auto derivation code like this
implicit def derive[A[_[_]]: FunctorK, F[_], G[_])(implicit af: A[F], fk: F ~> G): A[G] = af.mapK(fk)
With this given an algebra with FunctorK and an available interpreter, I can automatically derive an interpreter of any effect type as long as there is a FunctionK route from the available interpreter to this effect type.
This sort of auto derivation can only be achieved with a type class.

I agree with @kailuowang that it's not very useful in general as a typeclass atleast for cats-core. The only reason I could see, is that it might be useful for both cats-mtl and mainecoon which both depend on cats-core, so they don't have to define their own separately.

Quick reminder - StateT won't have an instance; it imposes a F: Functor constraint due to the F[S => F[(S, A)]] encoding.

@LukaJCB the need for a FunctorK in cats-mtl and mainecoon is actually different. In cats-mtl it took the shape of FunctorK[A[_[_], _]] until edmund decided it's not worth the abstraction, in mainecoon, it is required to have the shape of FunctorK[A[_[_]]], because that's the shape of final tagless encoded algebras.

Ah you're right! In that case, my vote goes to merging it as is right now :)
I can definitely empathize with @andyscott's situation, as I have wanted the exact same thing in the past 😄

@iravid Yep. Everything except IRWST and IndexedStateT could have an instance if we add FunctorK.

I think all feedback has been addressed. Codecov has failed the PR because the deprecated methods don't have coverage (they simply forward to mapK).

@andyscott it can also be added to

• EitherK (on the right F)
• Tuple2K (on the right F)
• Free
• Func
• Nested
• OneAnd
  Also I noticed that tests/src/test/scala/cats/tests/IndexedReaderWriterStateTTests.scala
  should've been renamed to tests/src/test/scala/cats/tests/IndexedReaderWriterStateTSuite.scala
  Let me know if you have time to add those , if not I'll be more than happy to submit a PR against your branch.

@johnynek shall we unify the methods first in this PR and determine the type class later. There have been several attempts to add this FunctorK type class, none of them reached the consensus. If anything that should tell us that we shouldn't rush it into cats.core 1.0.
On the other hand, these straightforward mapK methods have been needed and I don't see any harm of having them without the type class.

@kailuowang I'll take care of them within the next day.

I guess we can just copy this pattern all over.

I kind of hate that in a library about typeclasses we are ignoring this.

Also, I think it would compose in a nice way for the transformers. So, if you have OptionT[EitherT[F, A, B]] you could compose to point into something deeper.

Of course, the scala type system issues are a pain, so I guess if we don't have a clean way to write this typeclass we should punt.

There have been discussions on having a cats-more or extras module. While FunctorK doesn't provide a ton of utility to end users of cats, it would provide utility to other library authors (and anyone who wants to check the laws). Maybe the typeclass should go there?

see #1940

I'm actually -1 on tiny modules that we then have to bolt on a ton of boiler plate and imports.

FunctorK if we did it right would be a trivial increase in the size of cats. Having to add an additional import, build setup, implicit imports, etc... seems unmotivated to me.

The main motivations, in my view, for splitting modules are:

1. decrease jar size when many don't need everything. This should be done when we can save megabytes, not kilobytes.
2. reduce dependencies, so users who do not need a module don't complicate their build problem with additional dependencies.

I really don't think there are significant reasons to split other than that. This does not seem to be one of those two cases.

I'm also -1 on tiny modules for the same reasons. But if we can't add something to the core module and it's valuable, then having it in a tiny module is arguably better than not having it at all.

We should continue the discussion here #1713. And like @kailuowang said we could potentially still add it if we come to a good conclusion.

I've added missing instances. The remaining tests still need to be added.

When adding the method to Free, I realized I have it set up so mapK on Free operates on the free functor S but mapK on FreeT operates on the transformer context M. Should mapK for FreeT also operate on the functor S?

I am fine withmapK operates on S on Free and M on FreeT .

The Travis build fails because the Parallel instance for Kleisli uses the transform method which is now deprecated, you might need to rebase and change that one :)
Otherwise LGTM! 🎉

Actually can we also not deprecate hoist that's a common alias.

@andyscott we continued this in #2000 so that we can release this morning. Sorry for the rush. thanks for this awesome contribution.

closing as #2000 is merged